[Block Editor]: Try marking BlockList as non-Root component #35932
Conversation
ntsekouras
added
[Type] Bug
| @@ -17,7 +20,12 @@ export default function LiveBlockPreview( { onClick } ) { | |||
| onKeyPress={ onClick } | |||
| > | |||
| <Disabled> | |||
| <BlockList /> | |||
| <BlockList | |||
| renderAppender={ false } | |||
There was a problem hiding this comment.
This is not related to the actual problem, but noticed it here and is connected with the style removal here.
| @@ -17,7 +20,12 @@ export default function LiveBlockPreview( { onClick } ) { | |||
| onKeyPress={ onClick } | |||
| > | |||
| <Disabled> | |||
| <BlockList /> | |||
| <BlockList | |||
There was a problem hiding this comment.
Sorry for the late reply. It should be, yes. What's the difference between live and not live?
There was a problem hiding this comment.
live just uses BlockList whereas the auto preview scale the preview with the help of useResizeObserver.
|
Size Change: +36 B (0%) Total Size: 1.07 MB
ℹ️ View Unchanged
|
| @@ -158,6 +158,7 @@ export default function PostTemplateEdit( { | |||
| __experimentalOnClick={ () => | |||
| setActiveBlockContext( blockContext ) | |||
| } | |||
| __experimentalIsRootContainer={ false } | |||
There was a problem hiding this comment.
What about the additional "div" added by "BlockPreview" won't this break alignments as well? For the active post Id, there's no addition div if I'm not wrong.
There was a problem hiding this comment.
The parent div just fills all the available space from its parent container. What we want to avoid is the generated style for root-container:
.editor-styles-wrapper .block-editor-block-list__layout.is-root-container > * {
max-width: [content width];
margin-left: auto;
margin-right: auto;
}
There was a problem hiding this comment.
yes, but the problem is that if there's a block that is full aligned or wide aligned... inside the previewed blocks, they won't get the layout styles properly, so they won't show as expected.
There was a problem hiding this comment.
I'm not sure I understand why they shouldn't work. If a block has specific layout, it's styles should be rendered properly.
The problem seems to be that they don't work right now as the layout styles are not rendered in PostContent. This PR with the LivePreview PR in readonly state seems to do the trick.
I don't know if I missing something here..
There was a problem hiding this comment.
This is a video with both changes applied(isRootContainer and PostContent LivePreview)
Screen.Recording.2021-10-26.at.11.54.51.AM.mov
There was a problem hiding this comment.
Just compare the DOM output of an active "post id" with an inactive one, you'll see an extra div for the inactive one that disappears as soon as it becomes active.
Right now, in this PR it doesn't break anything because there's an "li" container for both situations that doesn't define any layout but if ever that "li" becomes say a "Post block" where you'd be able to define a "layout", it would break (it's the case in the other PR).
I'm fine if land this PR as is but since we need to solve the container-less preview anyway for the other PR, I was wondering if we could use the same technique here which would make the __experimentalIsRootContainer unnecessary.
Anyway not important for the Current PR as there's no layout in the container of the "preview"
There was a problem hiding this comment.
Right now, in this PR it doesn't break anything because there's an "li" container for both situations that doesn't define any layout but if ever that "li" becomes say a "Post block" where you'd be able to define a "layout", it would break
I get what you mean now.
I was wondering if we could use the same technique here which would make the __experimentalIsRootContainer unnecessary.
What technique are you referring to?
There was a problem hiding this comment.
What technique are you referring to?
I mean if we find a way to use BlockPreview without extra wrapper.
|
Since it seems the PR for handling the previews with the |
| function AutoBlockPreview( { | ||
| viewportWidth, | ||
| __experimentalPadding, | ||
| __experimentalIsRootContainer, |
There was a problem hiding this comment.
I'm hesitant about this prop, it's very weird to be honest. it's a hack because the issue here is not really the real issue here, the issue is the added div with its own layout. So while I'm ok with this PR, I was wondering if we can borrow the useBlockPreview from the other PR?
There was a problem hiding this comment.
Me too. I was kind of expecting to close it to be honest 😄
I was wondering if we can borrow the useBlockPreview from the other PR?
I'll try this out.
There was a problem hiding this comment.
I was wondering if we can borrow the useBlockPreview from the other PR?
Thanks for exploring that! I quite like the useBlockPreview we came up with in that other PR, so it'd be cool if we can include it here 🙂
There was a problem hiding this comment.
@andrewserong would you be interested in taking over this or maybe better creating a PR with useBlockPreview? I tried this a bit and it seems most of the code needed had been implemented by you in: #35863.
There was a problem hiding this comment.
@ntsekouras sure, I'm happy either take this over or create another PR. The only thing is I won't be able to start on it until later this week in case there's urgency in getting this in sooner. If not, then I can pick it up 🙂
There was a problem hiding this comment.
That sounds great, thanks! 💯 This is a bug, so there is time to fix it and include it in 5.9.
There was a problem hiding this comment.
I'm wrapping up for the week, but I've started on a fix and pushed a draft PR in #36431 that appears to work okay so far — I haven't had a chance to test it properly or see if it can be streamlined a bit, so I've left it as a draft PR for the moment, and will continue on with it on Monday.
There was a problem hiding this comment.
That's great Andrew! Thanks! I'll close this one and we can use your new PR.
noisysocks
added
the
Backport to WP 6.7 Beta/RC
ntsekouras
removed
the
Backport to WP 6.7 Beta/RC
Fixes: #33248
From the issue:
The problem
Currently all the
BlockPreviewsuse cases (ex Block previews in inserter, Styles, etc..) don't need to be aware of any parent alignments. The exception of that isQuery Loopand specifically thePost Templateblock (and soon theComments Loopblock) which internally use theBlockPreviewto achieve an emulation of how the final result will look like. I'm not sure if this fix is the right approach, but especially with patterns there might be more cases in the future? 🤔An alternative might be using override css, but I don't think it would scale well, as any change in the
root-containerstyles would also require change for the overrides.Testing instructions
Query Loopblock and setfull or widealignment.postsof theQuery Looprespect their parent alignment